✨ feat(tracker): Improve Tracker page UI/UX & add Repositories tab#658
✨ feat(tracker): Improve Tracker page UI/UX & add Repositories tab#658Mayank251125 wants to merge 1 commit into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
gssoc:approved
level:intermediate
quality:clean
type:accessibility
labels
📝 WalkthroughWalkthroughThis PR adds comprehensive GitHub repository tracking to the Tracker page. A new ChangesRepository Tracking Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @Mayank251125 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useGitHubRepos.ts`:
- Around line 57-68: The catch block in useGitHubRepos.ts swallowing errors
(inside fetchRepos) only calls setError and lets the promise resolve, so callers
like Tracker.tsx's handleSubmit (which uses toast.promise) incorrectly show
success; after each setError branch in the catch, propagate the failure by
re-throwing the original error (or return Promise.reject(err)) so fetchRepos
rejects and the caller can surface the correct toast/result—update the catch in
fetchRepos to throw err (or reject) after setError.
- Around line 49-56: The code incorrectly computes totalRepos by always
multiplying the rel="last" page number by perPage; change the logic in
useGitHubRepos to parse last page via lastMatch (keep linkHeader, lastMatch,
perPage, page, response.data.length, setTotalRepos) and compute total as
(lastPage - 1) * perPage + itemsOnLastPage, where itemsOnLastPage should be
response.data.length when the current page is the last page (lastPage === page)
and perPage otherwise; if you need exact totals even when the current page is
not the last, perform an extra fetch for the last page to read its length and
then setTotalRepos accordingly.
In `@src/pages/Tracker/Tracker.tsx`:
- Around line 615-626: The pagination effects currently read the mutable
`username` input, causing refetches to use unsaved edits; create and use a
committed `submittedUsername` state set by the search submit handler, then
change both effects (the tab/page effect that calls `fetchData` and the repoPage
effect that calls `fetchRepos`, and any other related effects around the 628-649
region) to read `submittedUsername` instead of `username` and include
`submittedUsername` in their dependency arrays; ensure the submit handler
assigns the current input to `submittedUsername` so all follow-up fetches
consistently use the last submitted username.
- Around line 428-442: The current filtering/sorting logic uses the paginated
array "repos" so search/filters (in the block producing "filtered") only run
against the current page; move the filtering/sorting to operate on the full
repository set (or fetch/cache all repos) before pagination is applied: ensure
the filter conditions referencing showForks, langFilter, search and the sort
logic using sort and sortDir are executed against the complete collection (e.g.,
"allRepos" or the fetched master list) and only then slice for the page, or
alternatively perform server-side filtering so the paginated "repos" is already
the filtered result.
- Around line 877-883: The tab elements rendered with Box currently only set
tabIndex on the active tab and handle Enter/Space but lack ArrowLeft/ArrowRight
roving focus; update the Tab keyboard handler to support left/right arrow
navigation by moving focus to the previous/next tab and calling setTab(newKey)
(wrapping at ends), and ensure tabIndex is 0 for the newly focused tab and -1
for others so focus moves correctly; modify the onKeyDown for the Box (and the
container with role="tablist" if present) to handle e.key ===
"ArrowRight"/"ArrowLeft" (and optionally Home/End) and also reset page/state via
setPage(0)/setStateFilter("all") when changing tabs to match existing click
behavior.
- Around line 444-455: The UI is showing page-only aggregates (totalStars,
totalForks computed from repos) but labels them as overall totals; update the
logic in Tracker.tsx so the summary uses the true overall totals or clearly
labels them as "this page": either (A) replace totalStars/totalForks with the
global totals source (if you have a backend/parent value like
totalStarsAll/totalForksAll or a paginatedTotals object) and use those in the
labels passed to fmtNum, or (B) change the label strings where
totalStars/totalForks (and the same occurrences around symbols at lines ~681-683
and ~855-863) are used to say "this page" (e.g., "stars on this page") so they
accurately reflect repos (the current page) rather than the entire dataset;
update every usage of totalStars and totalForks in Tracker.tsx accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f1699d0-178f-4270-8ff6-0ca376a08974
📒 Files selected for processing (2)
src/hooks/useGitHubRepos.tssrc/pages/Tracker/Tracker.tsx
| const response = await octokit.request('GET /users/{username}/repos', { | ||
| username, | ||
| per_page: perPage, | ||
| page, | ||
| sort: 'pushed', | ||
| direction: 'desc', | ||
| type: 'owner', | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the hook implementation around the referenced lines
sed -n '1,200p' src/hooks/useGitHubRepos.ts
# Inspect Tracker usage for toast.promise contract
sed -n '1,240p' src/pages/Tracker/Tracker.tsxRepository: GitMetricsLab/github_tracker
Length of output: 9576
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate toast.promise / handleSubmit usage in Tracker.tsx
rg -n "toast\.promise|handleSubmit|Promise\.all\(\[|fetchRepos\(" src/pages/Tracker/Tracker.tsx
# Print the relevant region around handleSubmit/toast.promise (limit output)
sed -n '1,260p' src/pages/Tracker/Tracker.tsx
sed -n '260,560p' src/pages/Tracker/Tracker.tsx
sed -n '560,920p' src/pages/Tracker/Tracker.tsxRepository: GitMetricsLab/github_tracker
Length of output: 37926
🌐 Web query:
GitHub REST API GET /users/{username}/repos private repositories authenticated visibility=all include private
💡 Result:
GitHub REST API does not support the exact endpoint/parameters in your query. 1) Endpoint mismatch - GET /users/{username}/repos is the endpoint for listing repositories for a specified user, but the REST API docs describe a limited set of query parameters (not a visibility=all style “include private” for that endpoint) and it’s not the authenticated-user listing endpoint. [1] 2) Correct endpoint for “include private” - To list both public and private repositories, you should use the “list repositories for the authenticated user” endpoint: GET /user/repos. [1] - That endpoint supports a visibility query parameter with allowed values all, public, private; default is all. [1] - Therefore, to include private repos, use visibility=private (or visibility=all) together with authentication. [1] 3) Authentication/scope requirement - Private repositories are returned only if the authenticated token/app has sufficient OAuth scope/permissions; generally GitHub requires appropriate repo/user permissions for private data. A related users endpoint notes that OAuth tokens need the repo scope to include private information. [2] Practical example (REST): - GET https://api.github.com/user/repos?visibility=all - Authorization: Bearer (must be authorized to access the private repos) [1][2] If you tell me whether you’re using a Personal Access Token, GitHub App installation token, or OAuth app token (and which scopes/permissions you have), I can recommend the exact visibility/affiliation/type filters and the right auth headers.
Citations:
- 1: https://docs.github.com/enterprise/2.2/developer/v3/repos/
- 2: https://docs.github.com/en/enterprise-cloud@latest/rest/users/users?apiVersion=2022-11-28
Fix repo fetching: private-repo endpoint, pagination count, and toast success/error contract
useGitHubReposrequestsGET /users/{username}/repos(withtype: 'owner'), which won’t include private repositories even with a PAT; switch to the authenticated-user endpoint (GET /user/repos) and passvisibility=all|private(with proper token scopes).const response = await octokit.request('GET /users/{username}/repos', { username, per_page: perPage, page, sort: 'pushed', direction: 'desc', type: 'owner', });
totalReposmultiplies therel="last"page number byperPage, which overestimates when the last page is partial.fetchReposswallows errors (setserrorstate but never rethrows), soTracker.tsx’stoast.promise(Promise.all([p1, p2]))can show the success message even when repos fail to load; makefetchReposreject on failure (or change toast to usereposError).
| const linkHeader = (response.headers as any)?.link ?? ''; | ||
| const lastMatch = linkHeader.match(/page=(\d+)>; rel="last"/); | ||
| const total = lastMatch | ||
| ? parseInt(lastMatch[1], 10) * perPage | ||
| : (page - 1) * perPage + response.data.length; | ||
|
|
||
| setRepos(response.data as GitHubRepo[]); | ||
| setTotalRepos(total); |
There was a problem hiding this comment.
totalRepos is wrong when the last page is not full.
Multiplying rel="last" by perPage overcounts for any user whose final page has fewer than perPage repositories. A user with 13 repos and perPage=12 will be reported as 24 here, which then breaks the repo stat card and pagination bounds.
🧰 Tools
🪛 ESLint
[error] 49-49: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useGitHubRepos.ts` around lines 49 - 56, The code incorrectly
computes totalRepos by always multiplying the rel="last" page number by perPage;
change the logic in useGitHubRepos to parse last page via lastMatch (keep
linkHeader, lastMatch, perPage, page, response.data.length, setTotalRepos) and
compute total as (lastPage - 1) * perPage + itemsOnLastPage, where
itemsOnLastPage should be response.data.length when the current page is the last
page (lastPage === page) and perPage otherwise; if you need exact totals even
when the current page is not the last, perform an extra fetch for the last page
to read its length and then setTotalRepos accordingly.
| } catch (err: any) { | ||
| const status = err?.status; | ||
| const message = err?.message?.toLowerCase() ?? ''; | ||
|
|
||
| if (status === 403) { | ||
| setError('GitHub API rate limit exceeded. Please provide a PAT to continue.'); | ||
| } else if (status === 404 || message.includes('not found')) { | ||
| setError('User not found. Please check the GitHub username.'); | ||
| } else if (status === 401) { | ||
| setError('Invalid token. Please check your Personal Access Token.'); | ||
| } else { | ||
| setError('Unable to fetch repositories. Please verify the username or network connection.'); |
There was a problem hiding this comment.
Propagate repo fetch failures back to the caller.
This catch path only updates local state, so fetchRepos still resolves successfully. In Tracker.tsx, handleSubmit wraps it in toast.promise, which means a repo-only failure still produces the success toast. Re-throw after setError(...) or return an explicit failure result so the caller can surface the right outcome.
🧰 Tools
🪛 ESLint
[error] 57-57: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useGitHubRepos.ts` around lines 57 - 68, The catch block in
useGitHubRepos.ts swallowing errors (inside fetchRepos) only calls setError and
lets the promise resolve, so callers like Tracker.tsx's handleSubmit (which uses
toast.promise) incorrectly show success; after each setError branch in the
catch, propagate the failure by re-throwing the original error (or return
Promise.reject(err)) so fetchRepos rejects and the caller can surface the
correct toast/result—update the catch in fetchRepos to throw err (or reject)
after setError.
| const filtered = repos | ||
| .filter((r) => { | ||
| if (!showForks && r.fork) return false; | ||
| if (langFilter !== "All" && r.language !== langFilter) return false; | ||
| if (search && | ||
| !r.name.toLowerCase().includes(search.toLowerCase()) && | ||
| !(r.description ?? "").toLowerCase().includes(search.toLowerCase())) return false; | ||
| return true; | ||
| }) | ||
| .sort((a, b) => { | ||
| const av = (a as any)[sort] ?? 0; | ||
| const bv = (b as any)[sort] ?? 0; | ||
| const cmp = typeof av === "string" ? av.localeCompare(bv) : av < bv ? -1 : av > bv ? 1 : 0; | ||
| return sortDir === "asc" ? cmp : -cmp; | ||
| }); |
There was a problem hiding this comment.
Repo search/filter/sort only works on the current page slice.
These controls run against the 12 repositories currently loaded into repos, not the user's full repository set. Once a user has multiple pages, searching or filtering can show "No repositories found" even when matching repos exist on later pages. This needs either all-repos fetching/caching or server-side filtering before exposing global search/filter UI.
🧰 Tools
🪛 ESLint
[error] 438-438: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 439-439: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Tracker/Tracker.tsx` around lines 428 - 442, The current
filtering/sorting logic uses the paginated array "repos" so search/filters (in
the block producing "filtered") only run against the current page; move the
filtering/sorting to operate on the full repository set (or fetch/cache all
repos) before pagination is applied: ensure the filter conditions referencing
showForks, langFilter, search and the sort logic using sort and sortDir are
executed against the complete collection (e.g., "allRepos" or the fetched master
list) and only then slice for the page, or alternatively perform server-side
filtering so the paginated "repos" is already the filtered result.
| const totalStars = repos.reduce((s, r) => s + r.stargazers_count, 0); | ||
| const totalForks = repos.reduce((s, r) => s + r.forks_count, 0); | ||
|
|
||
| return ( | ||
| <Box> | ||
| {/* Mini stat row */} | ||
| {!loading && repos.length > 0 && ( | ||
| <Box sx={{ display: "flex", gap: 3, mb: 2.5, flexWrap: "wrap" }}> | ||
| {[ | ||
| { icon: <BookOpen size={14} />, label: `${totalRepos} repositories` }, | ||
| { icon: <Star size={14} />, label: `${fmtNum(totalStars)} stars total` }, | ||
| { icon: <GitFork size={14} />, label: `${fmtNum(totalForks)} forks total` }, |
There was a problem hiding this comment.
These "total" repo metrics are page totals.
totalStars and totalForks are reduced from the current repos page, but the UI labels them as overall totals in both the repos header and the summary cards. For users with more than one repo page, those numbers are incorrect. Either aggregate across the full dataset or relabel them as "this page".
Also applies to: 681-683, 855-863
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Tracker/Tracker.tsx` around lines 444 - 455, The UI is showing
page-only aggregates (totalStars, totalForks computed from repos) but labels
them as overall totals; update the logic in Tracker.tsx so the summary uses the
true overall totals or clearly labels them as "this page": either (A) replace
totalStars/totalForks with the global totals source (if you have a
backend/parent value like totalStarsAll/totalForksAll or a paginatedTotals
object) and use those in the labels passed to fmtNum, or (B) change the label
strings where totalStars/totalForks (and the same occurrences around symbols at
lines ~681-683 and ~855-863) are used to say "this page" (e.g., "stars on this
page") so they accurately reflect repos (the current page) rather than the
entire dataset; update every usage of totalStars and totalForks in Tracker.tsx
accordingly.
| useEffect(() => { | ||
| if (username) { | ||
| if (prevFetched.current && username && tab !== "repos") { | ||
| fetchData(username, page + 1, ROWS_PER_PAGE); | ||
| } | ||
| }, [tab, page]); | ||
|
|
||
| const handleSubmit = (e: React.FormEvent<HTMLFormElement>): void => { | ||
| e.preventDefault(); | ||
| setPage(0); | ||
| fetchData(username, 1, ROWS_PER_PAGE); | ||
| }; | ||
| }, [tab, page]); // eslint-disable-line react-hooks/exhaustive-deps | ||
|
|
||
| const handlePageChange = (_: unknown, newPage: number) => { | ||
| setPage(newPage); | ||
| }; | ||
|
|
||
| const formatDate = (dateString: string): string => | ||
| new Date(dateString).toLocaleDateString(); | ||
| // NEW: re-fetch repos on repoPage change | ||
| useEffect(() => { | ||
| if (prevFetched.current && username && tab === "repos") { | ||
| fetchRepos(username, repoPage + 1, REPOS_PER_PAGE); | ||
| } | ||
| }, [repoPage]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Pagination is tied to the editable input, not the last submitted username.
After a successful search, the user can change the username field without submitting. The next page/tab refetch will then use that unsaved value, so pagination can silently switch accounts while the rest of the screen still reflects the previous search. Store a committed submittedUsername on submit and drive all follow-up fetches from that value.
Also applies to: 628-649
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Tracker/Tracker.tsx` around lines 615 - 626, The pagination effects
currently read the mutable `username` input, causing refetches to use unsaved
edits; create and use a committed `submittedUsername` state set by the search
submit handler, then change both effects (the tab/page effect that calls
`fetchData` and the repoPage effect that calls `fetchRepos`, and any other
related effects around the 628-649 region) to read `submittedUsername` instead
of `username` and include `submittedUsername` in their dependency arrays; ensure
the submit handler assigns the current input to `submittedUsername` so all
follow-up fetches consistently use the last submitted username.
1 participant
Related Issue
Description
This PR significantly improves the Tracker page UI/UX, accessibility, and overall user
experience as proposed in the linked issue. It also introduces a brand new
Repositories Tab that fetches and displays real GitHub repository data.
🎨 UI/UX Improvements
🚀 UX Improvements
📊 Summary Stat Cards
🗂️ New: Repositories Tab
GET /users/{username}/reposusing Octokittopics, license, visibility badge, fork badge, last updated (relative time)
useGitHubRepos.ts🔍 Filtering & Data Experience
♿ Accessibility
aria-labelon all inputs, buttons, icons, and interactive elementsrole="tab"andaria-selectedon tab pillsrole="status"on stat cards and empty states:focus-visiblerings on every interactive element<span role="img">📱 Responsiveness
maxWidthbreakpoints per columnHow Has This Been Tested?
torvalds,Mayank251125)npm run devruns without errorsScreenshots (if applicable)
Type of Change
Summary by CodeRabbit
Release Notes